Allow pyside2 gui loop #444
Conversation
| @register_integration('qt', 'qt5') | ||
| def loop_qt5(kernel): | ||
| """Start a kernel with PyQt5 event loop integration.""" | ||
| os.environ['QT_API'] = 'pyqt5' |
There was a problem hiding this comment.
If this is not set, then there's no way to select the right Python bindings to create the event loop (unless users set QT_API by hand).
Therefore, I'm -1 on this one.
There was a problem hiding this comment.
OK, changed it to set if unset. This also prevents it from being overwritten in every loop (as it was originally).
|
Ok, this is much better now, thanks! The only problem I see with this is that users still need to set @bergtholdt, do you think we could improve that situation? |
|
OK I changed it to use pyqt5 as default and pyside2 as fall-back. That should not affect anything that worked before. A little improvement, in the case that both backends are available, could be done to check if either module has been imported before calling %gui qt5 by checking sys.modules, but I would suspect that in that case the user sets the variable herself ("explicit is better than implicit"). |
Ok, let's go with this simple approach for now and see how users respond to it. |
|
Hey there, @bergtholdt, thank you for your contribution. I'm going through old PRs and this one has stalled. I'm going to power cycle this one (close it and reopen it) just so we get the latest set of tests to rerun on it, and once I hear back on my the last remaining question to @ccordoba12 (or anyone else knowledgeable in QT stuff), then we can merge it. |
my interpretation of suggestion from @ccordoba12 and @stukowski
|
@ccordoba12 and @stukowski - does 2d15ed6 look like I'm doing the thing you were talking about to keep with previous behavior? I'll wait to merge until I hear back or someone more competent shows up 😄 |
Yeah, I think it does. I mean, it fallbacks to setting So I'm ok with merging this one now. |
|
Perfect, thank you, Carlos! |
|
and thank you @bergtholdt for writing this up in the first place, apologies that it took so long to get merged in. |
No prob, and thanks to you for persevering until this PR was merged. |
4 participants
Removed writing to the environment inside GUI loop.
Fixes #398
Makes #399 obsolete.